PR_26180_OWNER_019a-contracts-schemas-obsolescence-audit#298
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b513b9af0d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | `src/shared/contracts/replayContracts.js` | Active browser/runtime dependency | 2 | 0 | 0 | 0 | 0 | No. Browser/runtime still references it; DB/API migration cannot silently replace it. | move to www/ | | ||
| | `src/shared/contracts/restoreSnapshotContract.js` | Active test/validation dependency | 0 | 0 | 2 | 0 | 0 | Not yet. It remains a validation contract until replacement tests/docs are updated. | keep temporarily | | ||
| | `src/shared/contracts/reviewRatingContract.js` | Active test/validation dependency | 0 | 0 | 2 | 0 | 0 | Not yet. It remains a validation contract until replacement tests/docs are updated. | keep temporarily | | ||
| | `src/shared/contracts/sharedStateContracts.js` | Active test/validation dependency | 0 | 0 | 1 | 0 | 0 | Not yet. It remains a validation contract until replacement tests/docs are updated. | keep temporarily | |
There was a problem hiding this comment.
Mark shared state contracts as browser-runtime
When this audit is used to drive the follow-up move, this row would leave sharedStateContracts.js in the test-only bucket even though it is imported by browser code: git grep in the reviewed tree finds www/src/shared/state/guards.js:9 and www/src/shared/state/normalization.js:10 importing ../contracts/sharedStateContracts.js. The www refs count and recommendation should reflect that runtime dependency, otherwise a future move/archive based on this report can break the shared state modules in the browser.
Useful? React with 👍 / 👎.
| | `src/shared/contracts/projectWorkspaceRuntimeContract.js` | Active test/validation dependency | 0 | 0 | 9 | 0 | 0 | Not yet. It remains a validation contract until replacement tests/docs are updated. | keep temporarily | | ||
| | `src/shared/contracts/publishContract.js` | Active test/validation dependency | 0 | 0 | 11 | 0 | 0 | Not yet. It remains a validation contract until replacement tests/docs are updated. | keep temporarily | | ||
| | `src/shared/contracts/releaseContract.js` | Active test/validation dependency | 0 | 0 | 12 | 0 | 0 | Not yet. It remains a validation contract until replacement tests/docs are updated. | keep temporarily | | ||
| | `src/shared/contracts/replayContracts.js` | Active browser/runtime dependency | 2 | 0 | 0 | 0 | 0 | No. Browser/runtime still references it; DB/API migration cannot silently replace it. | move to www/ | |
There was a problem hiding this comment.
Include the replay contract validation reference
If this row is followed to move replayContracts.js to www/, it misses the existing validation dependency in dev/tests/shared/ContractIndexValidation.test.mjs: that test lists replayContracts.js in legacySharedContractFiles and asserts src/shared/contracts/${fileName} exists. Because the report says dev/tests refs is 0 and recommends move to www/, the follow-up migration can break that validation unless this test is included in the migration plan.
Useful? React with 👍 / 👎.
| | Preserve protected dev workspace | PASS | No protected workspace files moved. | | ||
| | git diff --check | PASS | Passed. | | ||
| | npm run validate:canonical-structure | PASS | Passed with 0 blocking violations. | | ||
| | Required reports and ZIP | PASS | Reports generated under `dev/reports/`; ZIP generated under `dev/workspace/zips/`. | |
There was a problem hiding this comment.
Write the delta ZIP to the required tmp path
The repo instructions for BUILD packaging require the delta ZIP at <project folder>/tmp/<TASK_NAME>_delta.zip, but this checklist marks the ZIP requirement as passing while documenting dev/workspace/zips/ instead. Consumers and validators that look in the mandated tmp/ location will not find the required handoff artifact, so this should be regenerated/reported at the required path.
Useful? React with 👍 / 👎.
PR_26180_OWNER_019a-contracts-schemas-obsolescence-audit
Summary
Governance/report-only audit of every tracked file under:
src/shared/contracts/**src/shared/schemas/**No source files were moved or deleted.
Findings
Key decision: do not bulk-move contracts/schemas to
api/. The audit found browser/runtime dependencies, including dynamic browser schema loading fromwww/src/shared/toolbox/schemaOnlyToolPresetValidation.jsfor/src/shared/schemas/tools/<tool>.schema.json.Recommendation
www/src/shared/...or split browser-safe public contracts before API-only migration.api/src/...only after browser imports are separated.dev/or replaced.dev/or archive after Owner decision.Validation
git diff --checkPASSnpm run validate:canonical-structurePASSReports
dev/reports/PR_26180_OWNER_019a-contracts-schemas-obsolescence-audit_report.mddev/reports/PR_26180_OWNER_019a-contracts-schemas-obsolescence-audit_audit.csvdev/reports/PR_26180_OWNER_019a-contracts-schemas-obsolescence-audit_requirement-checklist.mddev/reports/PR_26180_OWNER_019a-contracts-schemas-obsolescence-audit_validation-report.mdZIP
dev/workspace/zips/PR_26180_OWNER_019a-contracts-schemas-obsolescence-audit_delta.zip